Skip to content

ci: add smart PR workflow automation#82

Closed
ndycode wants to merge 302 commits intomainfrom
ci/pr-workflow-automation
Closed

ci: add smart PR workflow automation#82
ndycode wants to merge 302 commits intomainfrom
ci/pr-workflow-automation

Conversation

@ndycode
Copy link
Copy Markdown
Owner

@ndycode ndycode commented Mar 14, 2026

Summary

  • add a required ci.yml workflow with smart change detection, required-pr, and workflow linting
  • add advisory coverage, compatibility, and dependency-audit lanes plus a PR governance workflow and docs link checker script
  • update contributor and testing docs to explain the required vs advisory CI lanes
  • follow up on workflow and docs-check review hardening, including repo metadata fallback, rename-aware change detection, live label reads, governance body sanitization/serialization, shortcut-reference parsing, and CommonMark fence stripping

Testing

  • npm run docs:check
  • npm run lint
  • npm run typecheck
  • npm run build
  • npm test
  • Not applicable

Docs Impact

  • README or docs updated
  • No docs changes needed

Compliance Confirmation

  • This change stays within the repository scope and OpenAI Terms of Service expectations.
  • This change uses official authentication flows only and does not add bypass, scraping, or credential-sharing behavior.
  • I updated tests and documentation when the change affected users, maintainers, or repository behavior.
  • No auth, request-routing, or storage paths changed.
  • I manually tested with a real ChatGPT Plus/Pro account.
  • Maintainers can apply the maintainer-live-verified label after independent live verification.

Notes

  • Linked issue: none.
  • Follow-up work or rollout notes: npm run test:coverage and npm run audit:ci remain advisory because the current baseline still fails the global coverage threshold and reports the existing hono audit issue. This PR also touches .github/pull_request_template.md, so the exempt label is needed for the existing pr-quality workflow.

Summary by CodeRabbit

Release Notes

  • Documentation

    • README updated with testing badge and additional contributing resources.
    • Expanded testing documentation with detailed test scenarios and maintainer verification requirements.
    • Improved documentation link validation.
  • Tests

    • Added comprehensive test suite for documentation link verification.
  • Chores

    • Enhanced CI/CD workflows with improved change detection and conditional job execution.
    • Updated PR templates and governance workflows for compliance tracking.

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

this PR introduces a complete CI/PR automation layer: a required ci.yml workflow with smart rename-aware change detection and a hardened required-pr aggregate gate, advisory pr-advisory.yml lanes for coverage, compat-matrix, and dependency audit, a pr-governance.yml workflow that validates the PR template and manages live-verification labels via pull_request_target, a docs-check.js link validator using only node built-ins, and accompanying docs/template updates.

  • all prior review thread issues are addressed: changes and detect-dependency-change both have timeout-minutes: 10, required-pr fails fast when CHANGES_RESULT != "success", HTML comments are stripped before link extraction, and scripts/ci/ changes now trigger docs_changed=true
  • pr-governance.yml correctly avoids checking out PR head ref under pull_request_target, uses live label reads via github.paginate, and serialises runs (cancel-in-progress: false) to prevent label race conditions — no windows filesystem or token-safety paths are touched by this PR
  • advisory lanes (coverage, compat-matrix, dependency-audit) lack an equivalent DETECT_RESULT guard: if detect-dependency-change fails or is cancelled, all three advisory jobs are silently skipped with no step summary written; since these are unrequired this does not block merge, but silent skips are hard to debug
  • getSectionContent in the governance script builds a RegExp from an unescaped heading string — benign with today's hardcoded callers but a latent injection risk if the function is reused
  • test/docs-check.test.ts retry cleanup is intentionally broad (retries all errors, not just Windows transient ones), which could leave orphaned tmp/docs-check-* dirs on Windows runners if the retry budget is exhausted by a non-transient error; low severity given runner teardown

Confidence Score: 4/5

  • safe to merge — no auth, request-routing, or storage paths changed; previous blocking issues from review threads are resolved
  • the required gate hardening (CHANGES_RESULT check, timeouts) and governance workflow correctness (pull_request_target without head checkout, live label reads, serialised concurrency) are all solid. the remaining issues are advisory-lane visibility and a latent regex-injection in a helper function — neither blocks the required CI path or introduces a token/filesystem safety risk on windows
  • pr-advisory.yml (silent skip on detect failure) and pr-governance.yml (unescaped regex in getSectionContent)

Important Files Changed

Filename Overview
.github/workflows/ci.yml new required CI workflow with smart change detection, conditional job gates, and a hardened required-pr aggregate check; previous thread issues (missing timeout, CHANGES_RESULT guard) are resolved
.github/workflows/pr-advisory.yml adds advisory coverage, compat-matrix, and dependency-audit lanes; detect-dependency-change has a timeout but downstream jobs lack a result guard — a failed detection job silently skips all advisory lanes with no summary output
.github/workflows/pr-governance.yml uses pull_request_target with issues: write to manage labels and validate PR template; correctly avoids checking out PR head ref; getSectionContent builds regex with unescaped heading string — benign today but latent if reused
scripts/ci/docs-check.js comprehensive markdown link checker using only Node built-ins; HTML comment stripping, shortcut-reference parsing, rename-aware change detection, and repo-slug fallback are all present and correct
test/docs-check.test.ts good coverage of unit and subprocess paths including Windows-specific EPERM/EBUSY retry logic; cleanup retries all errors (not just transient Windows ones), which could leave orphaned tmp dirs on Windows CI runners under unusual conditions
.github/pull_request_template.md adds typecheck, docs:check, Docs Impact section, and live-verification checklist items aligned with the new pr-governance workflow requirements
CONTRIBUTING.md updated to document the required vs advisory CI lane split and explain required-pr and pr-governance as the two branch-protection gates
package.json adds docs:check and audit:ci scripts; no auth, request-routing, or storage paths changed; dependency versions unchanged

Sequence Diagram

sequenceDiagram
    participant PR as Pull Request
    participant CH as changes job
    participant CJ as code jobs<br/>(lint/typecheck/build/unit-linux/unit-windows)
    participant DS as docs-sanity
    participant AL as actionlint
    participant RP as required-pr gate
    participant PG as pr-governance<br/>(pull_request_target)
    participant AD as advisory lanes<br/>(coverage/compat/audit)

    PR->>CH: trigger on pull_request
    CH->>CH: classify changed files<br/>(code / docs / workflow)
    CH-->>CJ: code_changed=true
    CH-->>DS: docs_changed=true
    CH-->>AL: workflow_changed=true
    CJ-->>RP: result
    DS-->>RP: result
    AL-->>RP: result
    CH-->>RP: CHANGES_RESULT (fail-fast if not success)
    RP->>RP: evaluate gates<br/>exit 1 on any failure
    Note over RP: branch-protection required check

    PR->>AD: trigger (advisory, not required)
    AD->>AD: detect-dependency-change
    AD-->>AD: coverage / compat-matrix / dependency-audit<br/>(continue-on-error: true)

    PR->>PG: pull_request_target<br/>(opened/edited/labeled/sync)
    PG->>PG: fetch live labels via paginate
    PG->>PG: strip code blocks + HTML comments
    PG->>PG: validate template sections + checkboxes
    PG->>PG: detect risky paths (auth/request/storage)
    PG-->>PR: add/remove needs-live-verification label
    PG-->>PR: fail if template incomplete
    Note over PG: branch-protection required check
Loading

Comments Outside Diff (4)

  1. scripts/ci/docs-check.js, line 244-248 (link)

    Tilde-fenced code blocks not stripped before link extraction

    extractMarkdownLinks only strips triple-backtick fences but not ~~~ (tilde) fences, which are valid CommonMark code blocks. any link inside a tilde-fenced block is still extracted and validated:

    ~~~bash
    [example](./missing-file.md)
    ~~~

    this would cause a false failure in docs-sanity for anyone using tilde fences. the stripMarkdownCode in pr-governance.yml correctly handles both fence characters — the same fix is needed here.

    |~~~)[\s\S]*?(?:```|~~~)/g, "\n")
    .replace(/[^\n]+`/g, "`code`");

    
    
  2. .github/workflows/pr-governance.yml, line 292-296 (link)

    core.summary.write() failure silences governance failures

    await core.summary.addRaw(...).write() is awaited before the core.setFailed call. if the GitHub step-summary API is transiently unavailable, write() throws, the exception propagates, and core.setFailed is never reached. the caller sees a generic network error rather than the compliance failures accumulated in failures[], making governance silent exactly when it should be loudest.

    wrapping the summary write in a try/catch with a fallback console.log preserves governance signal even when the summary endpoint is down:

    try {
      await core.summary.addRaw(summaryLines.join("\n")).write();
    } catch (summaryError) {
      console.warn("Failed to write step summary:", summaryError?.message ?? summaryError);
      console.log(summaryLines.join("\n"));
    }
    
    if (failures.length > 0) {
      core.setFailed(failures.join("\n"));
    }
  3. scripts/ci/docs-check.js, line 243-247 (link)

    non-greedy fence pattern can prematurely close code blocks

    the pattern (?:\{3}|{3})[\s\S]*?(?:`{3}|{3})` is non-greedy and doesn't track fence-char identity. if a fenced code block's body contains a run of exactly 3 backticks (common in contributing/readme docs that demonstrate markdown syntax), the non-greedy match closes early at that interior sequence and everything after it — still inside the real block — is exposed to link extraction. a broken-link reference that appears after the interior triple-backtick will be extracted and reported as a false failure.

    example that triggers it:

    ```
    Use fenced blocks like:
    ```yaml
    key: value
    ```
    The link [guide](./missing.md) is here.
    ```

    the regex closes the outer block at the inner ```, so [guide](./missing.md) is extracted and validated.

    pr-governance.yml's stripMarkdownCode already handles this correctly with a line-by-line state machine that tracks fence char and length. the same approach should be used here.

    no test case in docs-check.test.ts covers code blocks whose bodies contain 3 backtick/tilde sequences.

  4. .github/workflows/pr-advisory.yml, line 104-108 (link)

    No failure-propagation guard for detect-dependency-change

    unlike required-pr in ci.yml (which now correctly fails fast when CHANGES_RESULT != "success"), the advisory downstream jobs (coverage, compat-matrix, dependency-audit) only gate on the outputs, not on the job result. if detect-dependency-change is cancelled or errors out, all three advisory jobs are silently skipped with no indication in the run summary.

    since these lanes are explicitly advisory this won't break the required gate, but it does mean a hung or crashed detect-dependency-change job produces a deceptively clean advisory run. consider adding a needs.detect-dependency-change.result == 'success' guard to each advisory if: condition, or a dedicated advisory summary step, so transient runner failures are visible.

Fix All in Codex

Prompt To Fix All With AI
This is a comment left during a code review.
Path: pr-governance.yml
Line: 123-125

Comment:
**Unescaped heading string used in regex**

`getSectionContent` builds a `RegExp` from the raw `heading` argument without escaping it. today's callers pass hardcoded strings (`"Docs Impact"`, `"Compliance Confirmation"`) that contain no regex-special chars, so this is benign. but if the function is ever reused with an arbitrary heading string (e.g. one containing `.` or `()`), the regex will silently misfire.

```suggestion
            function getSectionContent(markdown, heading) {
              const escapedHeading = heading.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
              const headingPattern = new RegExp(`^## ${escapedHeading}\\s*$`, "m");
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .github/workflows/pr-advisory.yml
Line: 104-108

Comment:
**No failure-propagation guard for `detect-dependency-change`**

unlike `required-pr` in `ci.yml` (which now correctly fails fast when `CHANGES_RESULT != "success"`), the advisory downstream jobs (`coverage`, `compat-matrix`, `dependency-audit`) only gate on the outputs, not on the job result. if `detect-dependency-change` is cancelled or errors out, all three advisory jobs are silently skipped with no indication in the run summary.

since these lanes are explicitly advisory this won't break the required gate, but it does mean a hung or crashed `detect-dependency-change` job produces a deceptively clean advisory run. consider adding a `needs.detect-dependency-change.result == 'success'` guard to each advisory `if:` condition, or a dedicated advisory summary step, so transient runner failures are visible.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: test/docs-check.test.ts
Line: 18-33

Comment:
**Windows temp cleanup silently swallows errors after exhausting retries**

`cleanupTempRoot` retries on any error (not just transient EPERM/EBUSY/EACCES) and logs a warning on final failure — which is intentionally non-fatal. the asymmetry with `runDocsCheckSubprocess`, which only retries windows transient errors, means a genuine non-transient cleanup failure (e.g., permissions on a CI-managed runner dir) will silently leave orphaned `tmp/docs-check-*` dirs under the workspace. these are excluded from git and cleaned by the runner, but they could accumulate across retried CI jobs in the same workflow run. low severity given the `force: true` flag and runner teardown, but worth tracking if you see workspace bloat on the windows compat-matrix lane.

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: d4974b5

numman-ali and others added 30 commits November 25, 2025 13:58
BREAKING CHANGE: Major prompt engineering overhaul

This release brings full parity with Codex CLI's prompt selection:

Model-Specific Prompts:
- gpt-5.1-codex-max* → gpt-5.1-codex-max_prompt.md (117 lines, frontend design)
- gpt-5.1-codex*, codex-* → gpt_5_codex_prompt.md (105 lines, focused coding)
- gpt-5.1* → gpt_5_1_prompt.md (368 lines, full behavioral guidance)

Legacy GPT-5.0 → GPT-5.1 Migration:
- gpt-5-codex → gpt-5.1-codex
- gpt-5 → gpt-5.1
- gpt-5-mini, gpt-5-nano → gpt-5.1
- codex-mini-latest → gpt-5.1-codex-mini

New Features:
- ModelFamily type for prompt selection ("codex-max" | "codex" | "gpt-5.1")
- getModelFamily() function for model family detection
- Lazy instruction loading per model family
- Separate caching per model family
- Model family logged in request logs

Fixes:
- OpenCode prompt cache URL (main → dev branch)
- Multi-model session log detection in test script

Test Coverage:
- 191 unit tests (16 new for model family detection)
- 13 integration tests with family verification

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Changed config/full-opencode.json plugin from local file path to npm package name
- Added Highlights section to v4.0.0 changelog emphasizing Codex Max support

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add "Plugin Versioning & Updates" section early in README
- Explain why pinning is required (Bun lockfile pins "latest")
- Show upgrade workflow: change version in config, restart
- Provide cache-clearing command for stuck users
- Update all config examples to use @4.0.0
- Remove redundant "Updating the Plugin" section

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix orphaned function_call_output errors during summary/compaction
- Add GitHub API rate limit fallback for Codex instructions

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
**Fixed**:
- Compaction losing context: Only filter orphaned function_call_output items, preserve matched pairs
- Agent creation failing: Properly detect streaming vs non-streaming requests
- SSE/JSON response handling: Convert SSE→JSON for generateText(), passthrough for streamText()

**Added**:
- gpt-5.1-chat-latest model support (normalizes to gpt-5.1)

**Technical**:
- Capture original stream value before transformation
- API always gets stream=true, but response handling based on original intent
- Orphan detection: only remove function_call_output without matching function_call

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
## Added
- GPT 5.2 model family with 4 reasoning presets (low/medium/high/xhigh)
- Full image input support for all 16 models via modalities config
- GPT 5.2 model family in codex.ts with dedicated prompt handling

## Changed
- Model ordering: GPT 5.2 → Codex Max → Codex → Codex Mini → GPT 5.1
- Removed default presets without explicit reasoning suffix
- Updated @opencode-ai/plugin to ^1.0.150 (audit fix)
- Test script now uses local dist and includes GPT 5.2 tests

## Tests
- 193 unit tests + 16 integration tests (all passing)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…fix, and HTML version update

This release combines fixes and features from PRs #62, #63, and #64:

### Added
- "none" reasoning effort support for GPT-5.2 and GPT-5.1 general purpose models
- gpt-5.2-none and gpt-5.1-none model mappings and presets (now 18 total models)
- 4 new unit tests for "none" reasoning behavior (197 total tests)

### Fixed
- Orphaned function_call_output 400 API errors - now converts orphans to assistant
  messages to preserve context while avoiding API errors
- OAuth HTML version display updated from 1.0.4 to 4.1.0

### Technical Details
- getReasoningConfig() detects GPT-5.1 general purpose models and allows "none"
- Codex variants auto-convert "none" to "low" (or "medium" for Codex Mini)
- Orphan handling now works regardless of tools presence in request

Contributors: @code-yeongyu (PR #63), @kanemontreuil (PR #64), @ben-vargas (PR #62)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Auth reliability + error handling alignment + request integrity fixes
Remap activeIndex based on the selected account key to avoid unexpected account switching when deduplicating stored accounts. Add regression tests for storage normalization and oauth-success copy, and refresh docs/CI labeling.
Rename the npm package/bin to opencode-openai-codex-auth-multiaccount and repoint installer/config/docs to the fork repo. This keeps upstream lean while preserving the multi-account rotation feature set.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 15, 2026

@ndycode: Kicking off a fresh review of the current head now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 12 files

@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 15, 2026

@coderabbitai review
@greptile @cubic review

@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented Mar 15, 2026

@coderabbitai review
@greptile @cubic review

@ndycode I have started the AI code review. It will take a few minutes to complete.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 15, 2026

@ndycode: Kicking off a fresh review of the current head now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/workflows/pr-governance.yml">

<violation number="1" location=".github/workflows/pr-governance.yml:41">
P1: The markdown stripping is incomplete, so governance checkboxes can still be spoofed inside valid code syntax (e.g., `~~~` fenced blocks or multi-backtick code spans).</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread .github/workflows/pr-governance.yml Outdated
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 12 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/development/TESTING.md">

<violation number="1" location="docs/development/TESTING.md:42">
P3: Update the advisory-matrix description to include the Windows Node 18 lane so the testing guide matches the actual workflow.</violation>
</file>

<file name=".github/workflows/pr-governance.yml">

<violation number="1" location=".github/workflows/pr-governance.yml:40">
P1: Compliance checkbox validation can be bypassed via HTML comments because comment content is not stripped before regex checks.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread .github/workflows/pr-governance.yml Outdated
Comment thread docs/development/TESTING.md Outdated
@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 15, 2026

Addressed the remaining governance/docs review items in f305221 and re-ran the full local validation set:

  • npm run docs:check
  • npm run lint
  • npm run typecheck
  • npm run build
  • npm test

Reset the PR body back to the human-maintained summary for current head f305221.

@coderabbitai review
@greptile @cubic review

@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented Mar 15, 2026

Addressed the remaining governance/docs review items in f305221 and re-ran the full local validation set:

  • npm run docs:check
  • npm run lint
  • npm run typecheck
    ...

@ndycode I have started the AI code review. It will take a few minutes to complete.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 15, 2026

@ndycode: Thanks for addressing the remaining governance/docs review items in f305221 and for running the full local validation suite (docs:check, lint, typecheck, build, and test). I'll kick off a fresh review against the current head now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 12 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/workflows/pr-governance.yml">

<violation number="1" location=".github/workflows/pr-governance.yml:20">
P2: The concurrency key is too specific: including `github.event.action` prevents stale governance runs for the same PR from being canceled.</violation>
</file>

<file name="scripts/ci/docs-check.js">

<violation number="1" location="scripts/ci/docs-check.js:270">
P2: `extractMarkdownLinks` misses shortcut reference links (`[label]`), so those links are not validated and broken docs links can slip through.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread .github/workflows/pr-governance.yml Outdated
Comment thread scripts/ci/docs-check.js
@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 15, 2026

Addressed the shortcut-reference docs-check gap in d16b767 and re-ran the full local validation set:

  • npm run docs:check
  • npm run lint
  • npm run typecheck
  • npm run build
  • npm test

Refreshed the PR body for current head d16b767.

@greptile @cubic review

@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented Mar 15, 2026

Addressed the shortcut-reference docs-check gap in d16b767 and re-ran the full local validation set:

  • npm run docs:check
  • npm run lint
  • npm run typecheck
    ...

@ndycode I have started the AI code review. It will take a few minutes to complete.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 12 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/ci/docs-check.js">

<violation number="1" location="scripts/ci/docs-check.js:251">
P1: The shortcut-reference regex is too permissive and matches inline/full-reference links, which can cause false broken-link failures in docs-check.</violation>
</file>

<file name=".github/workflows/pr-governance.yml">

<violation number="1" location=".github/workflows/pr-governance.yml:20">
P2: Concurrency is partitioned by event action, so governance runs for the same PR can execute in parallel and race on label updates/state checks.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread scripts/ci/docs-check.js Outdated
Comment thread .github/workflows/pr-governance.yml Outdated
@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 15, 2026

Addressed the remaining governance/docs-check review items in 2681483:

  • serialized pr-governance per PR to avoid cross-action label races without canceling in-flight invalidation runs
  • tightened shortcut-reference parsing so inline/full-reference links do not false-match as shortcut references
  • added regression coverage for the parser edge case

Re-ran the full local validation set on the committed head:

  • npm run docs:check
  • npm run lint
  • npm run typecheck
  • npm run build
  • npm test

Reset the PR body back to the human-maintained summary for current head 2681483.

@coderabbitai review
@cubic-dev-ai
@greptile-apps review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 15, 2026

@ndycode: Thanks for addressing the remaining governance/docs-check items in 2681483 — the serialized concurrency for pr-governance to prevent label races, the tightened shortcut-reference parsing to avoid false-matches on inline/full-reference links, and the regression coverage for that parser edge case all look like solid hardening steps. I'll kick off a fresh review against the current head now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 15, 2026

Addressed the remaining fresh governance review item in 4d1be96:

  • hardened PR-body fenced-code parsing so longer CommonMark closing fences cannot leak compliance checkboxes through pr-governance

Re-ran the local validation set on the committed head:

  • npm run docs:check
  • npm run lint
  • npm run typecheck
  • npm run build
  • npm test

Reset the PR body back to the human-maintained summary for current head 4d1be96.

@coderabbitai review
@cubic-dev-ai
@greptile-apps review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 15, 2026

@ndycode: Thanks for addressing the fenced-code parsing hardening in 4d1be96 — closing the CommonMark longer-fence leak in pr-governance is a solid correctness fix. Confirmed you re-ran the full local validation suite (docs:check, lint, typecheck, build, and test). Kicking off a fresh review against the current head now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 245-247: The CI job invoking the docs-check script uses "npm run
docs:check", but docs-check.js contains an isDirectRun guard that checks
process.argv[1] against __filename so main() never runs when launched via npm;
update the workflow step that currently runs npm run docs:check to invoke the
script directly with Node (e.g., run node ./scripts/ci/docs-check.js) so the
isDirectRun check passes and main() executes, ensuring the markdown link/CI
badge validation actually runs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bad10035-39c7-467e-8c41-32849c519255

📥 Commits

Reviewing files that changed from the base of the PR and between ec4e852 and 4d1be96.

📒 Files selected for processing (6)
  • .github/workflows/ci.yml
  • .github/workflows/pr-advisory.yml
  • .github/workflows/pr-governance.yml
  • docs/development/TESTING.md
  • scripts/ci/docs-check.js
  • test/docs-check.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/docs-check.test.ts

Comment thread .github/workflows/ci.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

exempt Maintainer-approved override for PR quality screening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants